Fast and correct one-shot systems with a system registry resource #7999

Pascualex · 2023-03-09T11:22:21Z

Objective

Working with exclusive world access is not always easy: in many cases, a standard system or three is more ergonomic to write, and more modularly maintainable.
For small, one-off tasks (commonly handled with scripting), running an event-reader system incurs a small but flat overhead cost and muddies the schedule.
Certain forms of logic (e.g. turn-based games) want very fine-grained linear and/or branching control over logic.
SystemState is not automatically cached, and so performance can suffer and change detection breaks.
Fixes One-shot systems via Commands for scripting-like logic #2192.
Partial workaround for API for dynamic management of systems (adding and removing at runtime) #279.

Solution

Adds a SystemRegistry resource to the World, which stores initialized systems keyed by their SystemSet.
Allows users to call world.run_system(my_system) and commands.run_system(my_system), without re-initializing or losing state (essential for change detection).
Add a Callback type to enable convenient use of dynamic one shot systems and reduce the mental overhead of working with Box<dyn SystemSet>.
~~Allow users to run systems based on their SystemSet, enabling more complex user-made abstractions.~~

Status

Only allow the registry to run single systems, instead of whole sets.

Future work

Parameterized one-shot systems would improve reusability and bring them closer to events and commands. The API could be something like run_system_with_input(my_system, my_input) and use the In SystemParam.
We should evaluate unification of commands and one-shot systems since they are two different ways to run logic on demand over a World.

Prior attempts

This PR continues the work done in #4090.

No plugin exists for bevy_ecs, so this was the best place.

Only the commands specific to this system are applied, so we should de-emphasize this choice.

This reverts commit ba66966.

Pascualex · 2023-03-09T14:21:33Z

@alice-i-cecile I don't like/understand indexing systems by their sets. I like the idea of moving systems out of schedules and into a centralized system registry, but IMO it should only be possible to run them by type.

The use-case of running multiple systems on demand seems better handled by schedules than by this "run in linear registration order".

I know that system labels and sets have been merged into a single concept. So I'm not sure what could be an alternative to sets but with a 1-1 relationship with systems.

alice-i-cecile · 2023-03-09T17:16:22Z

IMO it should only be possible to run them by type.

Agreed here: we should only accept a impl Into<SystemTypeSet> and enforce uniqueness. I don't think there's a reasonable use case for "one shot systems but you have multiple distinct copies of the systems and want to keep their internal state distinct". In that case, the user should newtype them.

The use-case of running multiple systems on demand seems better handled by schedules than by this "run in linear registration order".

Strongly agreed.

Pascualex · 2023-03-10T12:10:54Z

I'd like to unify the following commands:

pub struct RunSystemCommand<...> {
    _phantom_marker: PhantomData<M>,
    system: S,
}

// formerly RunSystemsBySetCommand
pub struct RunCallback {
    pub callback: Callback,
}

RunCallback stores the SystemTypeSet and requires preregistration, while RunSystemCommand stores the system itself and handles system registration. I don't see a strong reason to employ different strategies here. We could have Callback implement IntoSystem or just have run_callback call run_system if they employed the same strategy.

I personally favor preregistration for explicitness on the fact that this is being stored and cached.

alice-i-cecile · 2023-03-10T12:20:59Z

crates/bevy_app/src/system_registry.rs

+use bevy_ecs::system::{Callback, SystemRegistryError};
+
+impl App {
+    /// Register a system with any number of [`SystemLabel`]s.


SystemLabel should be removed from the docs.

Yes, the docs are not up to date, I want to stabilize some things first :)

Pascualex · 2023-03-10T15:36:31Z

@alice-i-cecile I have another question. #4090 stablishes as future work:

Use this API to store all systems, regardless of whether or not they belong to a schedule, and solve #2777.

Does this still make sense? How would this work with multiple instances of a system inside a world? Should they share change detection or each have their own?

alice-i-cecile · 2023-03-10T16:55:09Z

I don't think that proposal makes sense any more :) Like you said, it doesn't play nice with multiple copies of systems, and we already have schedules for that.

Pascualex · 2023-03-10T17:18:38Z

Yes, sadly I started to feel the same way after thinking about some of the details involved.

Even if we renounce to this idea of a global system registry where all systems are stored, we may still want a solution for running systems on demand instead of polling. Kind of like schedules but for single systems.

To me it makes sense to call this single-system schedule-like things "Commands", that's what my proposal in #7707 was about.

I know that you said that these systems usually don't require exclusive access, but the fact that they are not scheduled is what removes the possibility of parallelization (unless we can think of some cool solution). Just like this system registry solution requires using commands or direct access to the world to run the systems, even for non-exclusive systems.

The current commands for entity and component management could be renamed to something like exclusive/direct/world commands. That way we'd have exclusive commands (which wouldn't have the overhead of systems) and system commands (which would have the ergonomics of systems).

Edit: Conceptually there would be two ways of running systems of demand, each for a different use-case and with different capabilities:

Schedules: allow to run a group of systems on demand handling collisions, ordering and optional parallelization.
Commands: allow to run a single system (or direct function call) on demand, with the possibility of passing some arguments.

alice-i-cecile · 2023-03-10T18:18:02Z

We really can't reuse the name Commands: that's already taken for Commands.

I agree that we should be able to run systems without pre-registration. I'm currently conflicted between:

Require a distinct method like run_uncached
Just cache it the first time the system is run.

The first is more explicit, the second is substantially more ergonomic.

crates/bevy_ecs/src/system/system_registry.rs

Pascualex · 2023-04-14T16:40:21Z

Sorry for not providing more updates. I was diving deep in Bevy internals but I started on a new position shortly after. I'll try to continue contributing in the future, but right now I don't have the time.

I'm adopting this ~~child~~ PR. # Objective - Working with exclusive world access is not always easy: in many cases, a standard system or three is more ergonomic to write, and more modularly maintainable. - For small, one-off tasks (commonly handled with scripting), running an event-reader system incurs a small but flat overhead cost and muddies the schedule. - Certain forms of logic (e.g. turn-based games) want very fine-grained linear and/or branching control over logic. - SystemState is not automatically cached, and so performance can suffer and change detection breaks. - Fixes #2192. - Partial workaround for #279. ## Solution - Adds a SystemRegistry resource to the World, which stores initialized systems keyed by their SystemSet. - Allows users to call world.run_system(my_system) and commands.run_system(my_system), without re-initializing or losing state (essential for change detection). - Add a Callback type to enable convenient use of dynamic one shot systems and reduce the mental overhead of working with Box<dyn SystemSet>. - Allow users to run systems based on their SystemSet, enabling more complex user-made abstractions. ## Future work - Parameterized one-shot systems would improve reusability and bring them closer to events and commands. The API could be something like run_system_with_input(my_system, my_input) and use the In SystemParam. - We should evaluate the unification of commands and one-shot systems since they are two different ways to run logic on demand over a World. ### Prior attempts - #2234 - #2417 - #4090 - #7999 This PR continues the work done in #7999. --------- Co-authored-by: Alice Cecile <[email protected]> Co-authored-by: Federico Rinaldi <[email protected]> Co-authored-by: MinerSebas <[email protected]> Co-authored-by: Aevyrie <[email protected]> Co-authored-by: Alejandro Pascual Pozo <[email protected]> Co-authored-by: Rob Parrett <[email protected]> Co-authored-by: François <[email protected]> Co-authored-by: Dmytro Banin <[email protected]> Co-authored-by: James Liu <[email protected]>

alice-i-cecile · 2023-09-19T20:51:34Z

Merged elsewhere!

I'm adopting this ~~child~~ PR. # Objective - Working with exclusive world access is not always easy: in many cases, a standard system or three is more ergonomic to write, and more modularly maintainable. - For small, one-off tasks (commonly handled with scripting), running an event-reader system incurs a small but flat overhead cost and muddies the schedule. - Certain forms of logic (e.g. turn-based games) want very fine-grained linear and/or branching control over logic. - SystemState is not automatically cached, and so performance can suffer and change detection breaks. - Fixes bevyengine#2192. - Partial workaround for bevyengine#279. ## Solution - Adds a SystemRegistry resource to the World, which stores initialized systems keyed by their SystemSet. - Allows users to call world.run_system(my_system) and commands.run_system(my_system), without re-initializing or losing state (essential for change detection). - Add a Callback type to enable convenient use of dynamic one shot systems and reduce the mental overhead of working with Box<dyn SystemSet>. - Allow users to run systems based on their SystemSet, enabling more complex user-made abstractions. ## Future work - Parameterized one-shot systems would improve reusability and bring them closer to events and commands. The API could be something like run_system_with_input(my_system, my_input) and use the In SystemParam. - We should evaluate the unification of commands and one-shot systems since they are two different ways to run logic on demand over a World. ### Prior attempts - bevyengine#2234 - bevyengine#2417 - bevyengine#4090 - bevyengine#7999 This PR continues the work done in bevyengine#7999. --------- Co-authored-by: Alice Cecile <[email protected]> Co-authored-by: Federico Rinaldi <[email protected]> Co-authored-by: MinerSebas <[email protected]> Co-authored-by: Aevyrie <[email protected]> Co-authored-by: Alejandro Pascual Pozo <[email protected]> Co-authored-by: Rob Parrett <[email protected]> Co-authored-by: François <[email protected]> Co-authored-by: Dmytro Banin <[email protected]> Co-authored-by: James Liu <[email protected]>

alice-i-cecile added 30 commits April 17, 2022 22:19

Basic functionality

3e6a8c5

commands.run_system

f9b16c2

Add SystemRegistry resource as part of CorePlugin

8e32c74

No plugin exists for bevy_ecs, so this was the best place.

Run system by TypeId

961af58

Fix doc links

d7ece72

run_system_by_type_id for World and Commands

f54db1f

Always flush commands from systems

4fc35e5

Only the commands specific to this system are applied, so we should de-emphasize this choice.

Always initialize SystemRegistry on the World

a9be933

Add test suite

fed175c

Doc improvements

b883bca

Store registered systems by their system labels

ee33630

Ensure that run_system always runs the system exactly once

b520291

Advertise SystemRegistry in SystemState docs

7041713

Doc tests for SystemRegistry

1a5e308

Improve ergonomics of the register_system API

6f3a676

Improve run_by_label API ergonomics

ebe2f29

Improve ergonomics of registering systems

3e41300

Note limitations

0f0a6dd

Remove the need to manually update archetypes

870b12d

Move SystemRegistry to a field on World

ba66966

Revert "Move SystemRegistry to a field on World"

5baac51

This reverts commit ba66966.

Clippy fixes

3be2a2e

Merge remote-tracking branch 'origin/main' into system_registry

4feb974

Clippy fix

4fe2937

System recursion should fail

38fd422

Documentation improvements

641f5df

Nicer error handling

854a805

Return index from register system methods

7f41af9

Fix off by one error

5c35c51

Reduce noisy returns

681a6fc

remove non-related clippy lint

ba23d05

alice-i-cecile added this to the 0.11 milestone Mar 9, 2023

fix doc tests

9085d50

alice-i-cecile added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events X-Controversial There is active debate or serious implications around merging this PR labels Mar 9, 2023

Pascualex added 3 commits March 10, 2023 12:37

remove run_systems_by_set

aca4998

remove run_systems_by_label from Commands

3319664

rename params to marker

68ba9bd

alice-i-cecile reviewed Mar 10, 2023

View reviewed changes

Pascualex mentioned this pull request Mar 13, 2023

One-shot systems through Commands and Schedules #7707

Closed

key systems by SystemId

98aa178

Pascualex commented Mar 13, 2023

View reviewed changes

crates/bevy_ecs/src/system/system_registry.rs Show resolved Hide resolved

shorten system registry function names

0a59b3b

alice-i-cecile requested changes Mar 13, 2023

View reviewed changes

crates/bevy_ecs/src/system/system_registry.rs Show resolved Hide resolved

alice-i-cecile added the S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! label Apr 14, 2023

alice-i-cecile removed this from the 0.11 milestone Jun 19, 2023

Trashtalk217 mentioned this pull request Jun 26, 2023

One Shot Systems #8963

Merged

Selene-Amanita closed this Sep 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fast and correct one-shot systems with a system registry resource #7999

Fast and correct one-shot systems with a system registry resource #7999

Pascualex commented Mar 9, 2023 •

edited

Loading

Pascualex commented Mar 9, 2023

alice-i-cecile commented Mar 9, 2023

Pascualex commented Mar 10, 2023

alice-i-cecile Mar 10, 2023

Pascualex Mar 10, 2023

Pascualex commented Mar 10, 2023

alice-i-cecile commented Mar 10, 2023

Pascualex commented Mar 10, 2023 •

edited

Loading

alice-i-cecile commented Mar 10, 2023

Pascualex commented Apr 14, 2023

alice-i-cecile commented Sep 19, 2023

Fast and correct one-shot systems with a system registry resource #7999

Fast and correct one-shot systems with a system registry resource #7999

Conversation

Pascualex commented Mar 9, 2023 • edited Loading

Objective

Solution

Status

Future work

Prior attempts

Pascualex commented Mar 9, 2023

alice-i-cecile commented Mar 9, 2023

Pascualex commented Mar 10, 2023

alice-i-cecile Mar 10, 2023

Choose a reason for hiding this comment

Pascualex Mar 10, 2023

Choose a reason for hiding this comment

Pascualex commented Mar 10, 2023

alice-i-cecile commented Mar 10, 2023

Pascualex commented Mar 10, 2023 • edited Loading

alice-i-cecile commented Mar 10, 2023

Pascualex commented Apr 14, 2023

alice-i-cecile commented Sep 19, 2023

Pascualex commented Mar 9, 2023 •

edited

Loading

Pascualex commented Mar 10, 2023 •

edited

Loading